Skip to content

Conversation

@prashantrakh
Copy link
Contributor

@prashantrakh prashantrakh commented Oct 16, 2025

Summary by CodeRabbit

  • New Features

    • Feature flags for many optional components (exporter, vault, integrations, backstage adapter, etc.), nginx/ingress support, and configurable nginx settings.
    • Added runtime client credentials generation and broader deployment configuration for integrations, exporter, vault, backstage-adapter, TF module service.
  • Chores

    • Release track/version bump and tooling/image upgrades.
    • Simplified default values and updated database/chart defaults.
    • Added Terraform-related ignore patterns.
  • Improvements

    • Extended runtime timeouts, debug mode and enhanced deployment logging/status checks.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Walkthrough

Adds Terraform variables and wiring for nginx and feature flags, updates the deployer image and tooling, bumps chart and component versions, alters PostgreSQL Helm release and PVC/security settings, generates new secrets and ingress wiring, removes some Makefile targets, and introduces expanded Helm values files.

Changes

Cohort / File(s) Summary
Repository ignores
/.gitignore
Adds Terraform ignore patterns: *.tfvars, *.tfstate, *.tfstate.*, .terraform/, .terraform.lock.hcl.
Deployer image Dockerfile
deployer-image/Dockerfile
Base switched to Debian 11, Terraform/YQ versions updated, additional tooling installed (yq, jq, kubectl, pip upgrades), ENV timeouts and DEBUG added, manifests/terraform copied into image, cleanup steps added.
Image versioning
deployer-image/Makefile
Default TRACK changed 1.12.1; RELEASE changed ${TRACK}.0${TRACK}.1.
Marketplace metadata & job
deployer-image/marketplace/manifests/application.yaml.template, deployer-image/marketplace/schema.yaml, deployer-image/marketplace/manifests/job.yaml.template
Bumps descriptor/publishedVersion to 4.1.2; job template adds env vars (WAIT_FOR_READY_TIMEOUT,TESTER_TIMEOUT,DEBUG) and changes deployer-config mount to /data/values.yaml with subPath.
Removed build targets
deployer-image/terraform/Makefile
Removes deps, build, and helm/package targets and associated copy/packaging steps.
Root Terraform wiring
deployer-image/terraform/main.tf, deployer-image/terraform/variables.tf
Adds nginx_config and enable_feature variables at root, wires them into module "stackgen", and refactors Helm provider syntax.
Module variables
deployer-image/terraform/modules/stackgen-installation/variables.tf
Bumps stackgen_version to 0.10.1, expands enable_feature into a multi-field object with defaults, and adds nginx_config (client_max_body_size).
Module logic & resources
deployer-image/terraform/modules/stackgen-installation/main.tf, .../database.tf
Adds random_id resources for app credentials, extends secrets with appcd_client_id/appcd_client_secret, adds rds_host, changes PVC access mode/storage class and security contexts, injects enable_ingress and nginx_config into rendered AppCD YAML, makes Temporal depend on PostgreSQL; PostgreSQL Helm repo/chart/version updated to Bitnami postgresql v18.0.15 and resource settings adjusted.
Helm values — appcd & temporal
deployer-image/terraform/values/appcd.yaml, .../appcd-final.yaml, deployer-image/terraform/values/temporal.yaml
Introduces extensive appcd feature flags, ingress/nginx settings, exporter/vault/integrations/backstage/deployment-manager sections, centralized pullPolicy and resources; temporal values simplified (many optional blocks removed) and SQL driver set to postgres12.
Deployment script
deployer-image/scripts/deploy.sh
Adds DEBUG-driven verbose logging, validates /data/values.yaml, extracts and redacts key values, improves terraform init/app logging and error handling, and adds post-deploy kubectl-based status/diagnostics when available.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer/CI
  participant TF as Terraform root
  participant Module as stackgen module
  participant Helm as Helm provider
  participant K8s as Kubernetes

  Dev->>TF: terraform apply (with nginx_config, enable_feature)
  TF->>Module: pass variables (nginx_config, enable_feature)
  Module->>Helm: render values (appcd YAML includes nginx, enable_ingress, secrets)
  Helm->>K8s: install/upgrade charts (postgresql, temporal, dex, appcd, ...)
  Helm->>K8s: create Secrets (appcd_client_id/secret), PVCs, Ingress rules
  K8s->>Dev: resources become available / readiness probes report status
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰
I knitted flags and charts with care,
Debian hopped in, versions in the air.
Secrets tucked, NGINX trimmed its waist,
Ingress doors swing wide in haste.
A rabbit cheers this infra-pace! 🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'publish new version of deployer' is vague and generic, lacking specificity about what changed or why this version is being released. Replace with a more descriptive title that highlights the primary changes, such as 'Update deployer to v2.1 with Terraform/kubectl/PostgreSQL upgrades and security fixes' or 'Upgrade dependencies and address CVEs in deployer image'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch AE-1241/fix-vulnerabilties-d

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
deployer-image/terraform/modules/stackgen-installation/main.tf (2)

223-237: Hardcoding enable_ingress = true ignores module input.

Use var.enable_ingress so callers can disable ingress.

-    enable_ingress : true
+    enable_ingress : var.enable_ingress

158-166: SCM secret created with possibly empty data.

If scm_configuration does not match github/gitlab/azuredev, local.final_scm_secrets is {}, and creating a Secret with empty data can fail. Gate resource on presence of keys.

-resource "kubernetes_secret" "appcd_scm_secrets" {
+resource "kubernetes_secret" "appcd_scm_secrets" {
+  count = length(keys(local.final_scm_secrets)) > 0 ? 1 : 0
   depends_on = [kubernetes_namespace.this]
   metadata {
-    name      = "appcd-scm-secrets"
-    namespace = var.namespace
+    name      = "appcd-scm-secrets"
+    namespace = var.namespace
   }
-  data = local.final_scm_secrets
+  data = local.final_scm_secrets
 }

Also adjust references to this secret to handle count (e.g., only include when present).

deployer-image/terraform/values/temporal.yaml (1)

218-227: Fix invalid Ingress host and annotation usage

  • Host must not include a path. Use hosts: ["${domain}"] and define /internal/workload under a paths: block.
  • Remove nginx.org/mergeable-ingress-type: minion/location-snippets unless you pair them with a master Ingress for the NGINX Inc controller.
  • For community ingress-nginx, replace with nginx.ingress.kubernetes.io/rewrite-target and path-based routing.
🧹 Nitpick comments (6)
deployer-image/terraform/values/temporal.yaml (1)

28-31: Hard-coded ServiceMonitor label risks non-discovery.

Label release: kube-prometheus-stack may not match your Prometheus stack release. Make this configurable or derive from var to avoid silent non-scrape.

-      additionalLabels:
-        release: kube-prometheus-stack
+      additionalLabels:
+        release: ${prometheus_release_label}
deployer-image/terraform/values/appcd-final.yaml (1)

1-4: Avoid committing environment-specific rendered values.

This looks like a rendered artifact (final values for test.stackgen.local). Consider generating it in CI or .gitignoring it to prevent drift and accidental reuse in other environments.

Also applies to: 24-27, 49-62, 88-94, 95-117, 118-143, 144-159, 160-168, 165-174

deployer-image/terraform/values/appcd.yaml (2)

103-111: Probes disabled for iac-gen — reduces resilience.

Disabling liveness/readiness can mask crashes and slow startups. Recommend enabling with conservative thresholds.

   probes:
-    liveness:
-      enabled: false
-    readiness:
-      enabled: false
+    liveness:
+      enabled: true
+      initialDelaySeconds: 30
+      periodSeconds: 10
+      timeoutSeconds: 5
+      failureThreshold: 6
+    readiness:
+      enabled: true
+      initialDelaySeconds: 10
+      periodSeconds: 10
+      timeoutSeconds: 5
+      failureThreshold: 6

24-27: Duplicated Temporal config blocks.

You set both global.temporal and top-level temporal. If only one is consumed by charts, drop the duplicate to avoid drift.

Also applies to: 111-113

deployer-image/terraform/modules/stackgen-installation/main.tf (2)

73-95: Hardcoded GHCR dockerconfig values.

"username": "github_username" and "email": "support" are placeholders. Consider passing via variables and masking in state. Also prefer stringData for clarity.

-  data = {
+  string_data = {
     ".dockerconfigjson" = jsonencode({
       "auths" = {
         "https://ghcr.io" = {
-          "username" = "github_username"
+          "username" = var.ghcr_username
           "password" = var.STACKGEN_PAT
-          "email"    = "support"
+          "email"    = var.ghcr_email
           "auth"     = base64encode("${var.ghcr_username}:${var.STACKGEN_PAT}")
         }
       }
     })
   }

2-9: Pinning temporal_helm_version in locals reduces flexibility.

Expose as a variable with default to ease upgrades/rollbacks.

variable "temporal_helm_version" {
  type    = string
  default = "0.57.0"
}

Use var.temporal_helm_version in locals.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7dce41e and 6806f97.

📒 Files selected for processing (14)
  • .gitignore (1 hunks)
  • deployer-image/Dockerfile (2 hunks)
  • deployer-image/Makefile (1 hunks)
  • deployer-image/marketplace/manifests/application.yaml.template (1 hunks)
  • deployer-image/marketplace/schema.yaml (1 hunks)
  • deployer-image/terraform/Makefile (0 hunks)
  • deployer-image/terraform/main.tf (2 hunks)
  • deployer-image/terraform/modules/stackgen-installation/database.tf (2 hunks)
  • deployer-image/terraform/modules/stackgen-installation/main.tf (8 hunks)
  • deployer-image/terraform/modules/stackgen-installation/variables.tf (2 hunks)
  • deployer-image/terraform/values/appcd-final.yaml (1 hunks)
  • deployer-image/terraform/values/appcd.yaml (2 hunks)
  • deployer-image/terraform/values/temporal.yaml (4 hunks)
  • deployer-image/terraform/variables.tf (1 hunks)
💤 Files with no reviewable changes (1)
  • deployer-image/terraform/Makefile
🔇 Additional comments (8)
.gitignore (1)

1-6: Solid Terraform ignore additions.
Covers the usual tfvars, state, cache, and lock artifacts so they stay out of git. Nicely done.

deployer-image/terraform/values/temporal.yaml (2)

90-97: Double-check metrics scraping across all services.

server.metrics.serviceMonitor is wired to ${enable_ops}, but per-service blocks have empty serviceMonitor objects. Confirm the chart enables monitors for frontend/history/matching/worker when enable_ops is true, or explicitly enable them to ensure coverage.

Also applies to: 112-119, 133-140, 155-161


61-61: No issue found. The driver: "postgres12" value is correct for Temporal Helm chart 0.57.0. Both the Temporal Helm charts README and self-hosted documentation confirm "postgres12" as the correct SQL driver value for PostgreSQL persistence configuration.

deployer-image/terraform/values/appcd.yaml (2)

116-119: WORM storage tied to artifacts_support flag — verify semantics.

iac-gen.storage.worm.enabled uses ${enable_feature.artifacts_support}. Confirm this is intentional (artifact support enabling WORM) vs. an independent toggle.


5-11: nginx config passthrough: ensure template keys align.

Values read ${nginx.client_max_body_size}. Confirm var.nginx_config has client_max_body_size and the chart actually consumes nginx.* keys; otherwise this block is a no-op.

deployer-image/terraform/modules/stackgen-installation/main.tf (3)

65-72: Client ID/secret generation looks good.

random_id for appcd_client_id/secret and wiring into appcd-secrets is sound.

Also applies to: 119-128


42-53: Dex chart bump to 0.19.1

Version bump looks fine; values mapping unchanged.


182-198: Temporal depends_on includes Postgres — good ordering.

Ensures DB exists before chart install.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
deployer-image/Dockerfile (3)

47-47: Pin pip dependencies for reproducible builds.

Upgrading pip/setuptools/wheel to "latest" without version constraints makes builds non-reproducible and harder to audit. Consider specifying versions or at least a known-good baseline compatible with your environment:

- python3 -m pip install --upgrade pip setuptools wheel --no-cache-dir && \
+ python3 -m pip install --upgrade pip==24.3.1 setuptools==75.1.0 wheel==0.42.0 --no-cache-dir && \

Verify these versions address CVE-2024-35195 and CVE-2025-47273 before pinning.


9-11: Address yq version mismatch between stages.

Stage 1 (test_schema_creator) uses Alpine's default yq version, while Stage 2 explicitly pins YQ_VERSION=4.44.6. This inconsistency could lead to schema processing differences. Either pin yq in Stage 1 or document why different versions are acceptable:

- RUN apk add --no-cache yq && \
+ RUN apk add --no-cache yq==${YQ_VERSION} && \

Or, if Alpine's yq version differs, ensure your schema processing logic is version-agnostic.


14-14: Verify base image stability and deprecation path.

Line 14 updates to Debian 11 (good for security patches), and line 25 retains the :latest tag for deployer_envsubst. While Debian 11 receives long-term support, the :latest tag in Stage 2 creates a reproducibility gap. Consider pinning to a specific image digest or version tag if available from Google Cloud Marketplace.

Also applies to: 25-25

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6806f97 and f8d70d8.

📒 Files selected for processing (1)
  • deployer-image/Dockerfile (2 hunks)
🔇 Additional comments (1)
deployer-image/Dockerfile (1)

30-52: Good security and cleanup practices.

The expanded install block with system upgrades, security-focused package selection, and cleanup (removing wget/unzip and apt lists) effectively reduces the image attack surface and aligns with container security best practices. The explicit pip upgrade also addresses known CVEs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
deployer-image/Dockerfile (1)

30-52: Add checksum verification for Terraform and yq downloads.

Terraform and yq binaries are downloaded without integrity verification (checksums or GPG signatures) at lines 40–44. This reintroduces the MITM attack vector flagged in a previous review and leaves the build vulnerable to supply-chain compromise. Add verification to match HashiCorp and yq security best practices:

  # Install Terraform
  wget https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip && \
+ wget https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_SHA256SUMS && \
+ sha256sum -c terraform_${TERRAFORM_VERSION}_SHA256SUMS | grep linux_amd64 && \
  unzip terraform_${TERRAFORM_VERSION}_linux_amd64.zip -d /usr/local/bin/ && \
  rm terraform_${TERRAFORM_VERSION}_linux_amd64.zip && \
  # Install yq with specific version
  wget -O /usr/local/bin/yq https://github.com/mikefarah/yq/releases/download/v${YQ_VERSION}/yq_linux_amd64 && \
+ wget -O /usr/local/bin/yq.sha256 https://github.com/mikefarah/yq/releases/download/v${YQ_VERSION}/yq_linux_amd64.sha256 && \
+ sha256sum -c /usr/local/bin/yq.sha256 && \
  chmod +x /usr/local/bin/yq && \
  rm /usr/local/bin/yq.sha256 && \

Positive notes: The addition of apt-get upgrade (line 32) to apply security patches and the pip upgrade (line 47) to fix CVE-2024-35195 and CVE-2025-47273 are good security improvements. The comprehensive cleanup block (lines 49–52) properly removes build tools and APT cache.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8d70d8 and 82033e5.

📒 Files selected for processing (1)
  • deployer-image/Dockerfile (3 hunks)
🔇 Additional comments (1)
deployer-image/Dockerfile (1)

14-14: Base image security update.

Upgrading from debian10 to debian11 improves security posture and provides access to more recent packages and patches.

@prashantrakh prashantrakh marked this pull request as draft November 10, 2025 10:22
@prashantrakh prashantrakh marked this pull request as ready for review January 10, 2026 04:08
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
deployer-image/scripts/deploy.sh (1)

51-70: Error handling is ineffective due to set -e causing script exit before $? checks execute.

With set -e enabled (line 2), when terraform init fails at line 52, the script exits immediately without reaching the error check at lines 54–57. Similarly, if terraform apply fails at lines 61–67, the script exits before line 69 can capture the exit code, making the conditional logic at lines 70–135 unreachable.

Use structured error handling with if ! to allow proper error capture within the set -e context:

Proposed fix
 echo "[INFO] Initializing Terraform"
-terraform init -input=false
-
-if [ $? -ne 0 ]; then
+if ! terraform init -input=false; then
   echo "[ERROR] Terraform initialization failed!"
   exit 1
 fi

For terraform apply, capture the exit code before set -e terminates:

 echo "[INFO] Applying Terraform configuration"
 echo "[DEBUG] Terraform version: $(terraform version | head -1)"
-terraform apply \
+EXIT_CODE=0
+terraform apply \
   -var "suffix=${SUFFIX}" \
   -var "domain=${DOMAIN}" \
   -var "STACKGEN_PAT=${STACKGEN_PAT}" \
   -var "pre_shared_cert_name=${PRE_SHARED_CERT_NAME}" \
   -var "global_static_ip_name=${GLOBAL_STATIC_IP_NAME}" \
-  -auto-approve
-
-EXIT_CODE=$?
+  -auto-approve || EXIT_CODE=$?
🤖 Fix all issues with AI agents
In @deployer-image/Dockerfile:
- Around line 78-80: The Dockerfile currently sets ENV DEBUG=true which enables
verbose logging by default and risks leaking secrets; change the default to
false and require explicit opt-in (e.g., introduce ARG DEBUG=false and set ENV
DEBUG from that ARG or change ENV DEBUG to "false"), update any
documentation/CI/build scripts that rely on DEBUG to explicitly pass true when
needed, and ensure callers know to set DEBUG=true only for troubleshooting to
avoid accidental exposure.
- Around line 60-61: The Dockerfile comment incorrectly attributes
CVE-2024-35195 to pip; update the comment above the existing python3 -m pip
install --upgrade pip setuptools wheel --no-cache-dir line to state that
upgrading setuptools addresses CVE-2025-47273 and does not remediate
CVE-2024-35195, and if you intend to remediate CVE-2024-35195 also, add a
separate pip install step to upgrade the requests package (e.g., install
requests>=2.32.0) after the existing upgrade; ensure the comment and/or added
step clearly distinguishes which package addresses which CVE.
- Around line 47-59: The Dockerfile currently fetches a single "stable" kubectl
(KUBECTL_VERSION) but then copies that binary into versioned paths
(/opt/kubectl/1.30 and /opt/kubectl/1.31), which is misleading; either download
and verify each desired version explicitly (fetch distinct URLs/checksums per
version and place them into /opt/kubectl/1.30/kubectl and
/opt/kubectl/1.31/kubectl, keeping the existing sha256 verification per file) or
stop using versioned directories and name them /opt/kubectl/stable to match
KUBECTL_VERSION, and update the cp /usr/local/bin/kubectl step accordingly; also
update the top comment about CVEs to remove blanket claims and instead state the
exact conditions (e.g., “these CVEs may affect binaries built with vulnerable
Go/oauth2/go-getter versions; ensure the downloaded kubectl build metadata or Go
toolchain is fixed, or verify by using explicit versioned releases”) so the
comment accurately reflects when those CVEs apply.

In @deployer-image/marketplace/schema.yaml:
- Around line 6-8: The releaseNote under publishedVersionMetadata still reads
"Initial release with Job support." but publishedVersion is "4.1.2"; update
publishedVersionMetadata.releaseNote to accurately describe what changed or is
included in v4.1.2 (e.g., features, bug fixes, or API changes), ensuring the
text references version 4.1.2 and summarizes its actual contents rather than
saying "Initial release."

In @deployer-image/scripts/deploy.sh:
- Around line 91-103: The pod readiness check in deploy.sh incorrectly uses a
logical AND between "$ready" and "$phase", so diagnostics only run when the pod
is neither Ready nor Running; change the condition in the if that checks ready
and phase to use OR (i.e., replace && with ||) so that when either ready !=
"True" OR phase != "Running" the script prints describe/logs for the pod
(references: variables pod, phase, ready and the if block that echoes "[WARNING]
Pod $pod is not ready. Showing details:").
- Around line 4-8: The script enables shell debug (set -x) which will leak
STACKGEN_PAT when terraform commands echo CLI args; to fix, stop exporting or
revealing the secret by: temporarily disable xtrace around exporting/setting the
secret (use "set +x" before and "set -x" after the export) and stop passing
STACKGEN_PAT via a -var CLI argument in the terraform apply and terraform plan
invocations—instead set TF_VAR_STACKGEN_PAT as an environment variable (export
TF_VAR_STACKGEN_PAT="$STACKGEN_PAT") so the secret is injected into Terraform
without appearing in the command line; apply the same temporary set +x / set -x
protection and TF_VAR_STACKGEN_PAT change for both the terraform apply block and
the terraform plan error-handling path.
🧹 Nitpick comments (2)
deployer-image/scripts/deploy.sh (2)

31-35: Harden yq extraction: quote file, use raw strings, and default away from null.
As-is, missing keys can become literal null, and string values may include quotes/newlines depending on yq.

Proposed fix (more predictable parsing)
-SUFFIX=${SUFFIX:-$(yq '.suffix' $VALUES_FILE)}
-DOMAIN=${DOMAIN:-$(yq '.domain' $VALUES_FILE)}
-STACKGEN_PAT=${STACKGEN_PAT:-$(yq '.stackgenPat' $VALUES_FILE)}
-PRE_SHARED_CERT_NAME=${PRE_SHARED_CERT_NAME:-$(yq '.pre_shared_cert_name' $VALUES_FILE)}
-GLOBAL_STATIC_IP_NAME=${GLOBAL_STATIC_IP_NAME:-$(yq '.global_static_ip_name' $VALUES_FILE)}
+SUFFIX=${SUFFIX:-$(yq -r '.suffix // ""' "$VALUES_FILE")}
+DOMAIN=${DOMAIN:-$(yq -r '.domain // ""' "$VALUES_FILE")}
+STACKGEN_PAT=${STACKGEN_PAT:-$(yq -r '.stackgenPat // ""' "$VALUES_FILE")}
+PRE_SHARED_CERT_NAME=${PRE_SHARED_CERT_NAME:-$(yq -r '.pre_shared_cert_name // ""' "$VALUES_FILE")}
+GLOBAL_STATIC_IP_NAME=${GLOBAL_STATIC_IP_NAME:-$(yq -r '.global_static_ip_name // ""' "$VALUES_FILE")}

129-133: Be careful printing terraform show/plan to logs (can be sensitive + very large).
Even if some values are marked sensitive, state/plan output can still expose details you don’t want in CI logs; consider gating these behind DEBUG=true or a separate DIAGNOSTICS=true.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 82033e5 and bb85b3a.

📒 Files selected for processing (5)
  • deployer-image/Dockerfile
  • deployer-image/marketplace/manifests/application.yaml.template
  • deployer-image/marketplace/manifests/job.yaml.template
  • deployer-image/marketplace/schema.yaml
  • deployer-image/scripts/deploy.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • deployer-image/marketplace/manifests/application.yaml.template
🔇 Additional comments (5)
deployer-image/marketplace/manifests/job.yaml.template (2)

28-33: Confirm these new timeout/debug env vars are actually consumed and typed correctly.

If /data/deploy.sh (or downstream tooling) expects ints/seconds, verify WAIT_FOR_READY_TIMEOUT="900", TESTER_TIMEOUT="1200", and DEBUG="false" are parsed as intended and that overriding image defaults is desired.


36-37: No action needed—paths are already correctly aligned.

The deploy.sh script already expects the values file at /data/values.yaml (line 23: VALUES_FILE="/data/values.yaml"). The mount path in job.yaml.template (mountPath: /data/values.yaml) matches this exactly. There is no breaking change here; the script and configuration are aligned.

Likely an incorrect or invalid review comment.

deployer-image/Dockerfile (3)

14-14: Good update to Debian 11 base image.

Updating from Debian 10 (buster, EOL) to Debian 11 (bullseye) is a solid security improvement.


30-46: Good security practices in package installation and cleanup.

Solid approach: upgrading system packages, pinning tool versions, and removing temporary utilities (wget, unzip, curl) after use to reduce attack surface. The apt cache cleanup is also good for minimizing image size.

Also applies to: 62-66


70-76: Timeout increases are reasonable and well-documented.

The increased timeouts (10 min for readiness, 15 min for tester) accommodate longer deployment scenarios. Good documentation explaining the rationale.

Comment on lines +47 to +59
# Install latest kubectl versions to fix CVE-2025-8959, CVE-2025-61729, and CVE-2025-22868
# These vulnerabilities affect kubectl binaries built with vulnerable Go dependencies
# Installing latest kubectl versions that were built with fixed Go 1.22.6+ and updated dependencies
KUBECTL_VERSION=$(curl -L -s https://dl.k8s.io/release/stable.txt) && \
mkdir -p /opt/kubectl/1.30 /opt/kubectl/1.31 && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl.sha256" && \
echo "$(cat kubectl.sha256) kubectl" | sha256sum --check && \
chmod +x kubectl && \
cp kubectl /opt/kubectl/1.30/kubectl && \
cp kubectl /opt/kubectl/1.31/kubectl && \
cp kubectl /usr/local/bin/kubectl && \
rm kubectl kubectl.sha256 && \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

CVE-2025-8959 kubectl Go vulnerability

💡 Result:

Short answer
CVE-2025-8959 is a symlink/traversal vulnerability in HashiCorp’s go-getter library (affects go-getter <= 1.7.8) that can allow unauthorized reads outside a requested subdirectory. It was fixed in go-getter 1.7.9. This is a vulnerability in the library—not in kubectl itself—so only Kubernetes tooling or wrapper projects that embed or vendor go-getter (or depend on packages that do) are affected; plain upstream kubectl is not implicated unless your kubectl build or related tools include go-getter. [1][2][3]

What to do

  • If you maintain software that vendors or imports github.com/hashicorp/go-getter, upgrade to go-getter >= 1.7.9. [1][3]
  • If you use third-party Kubernetes tools (e.g., scanners, installers, CLIs like tools that fetch subdirectories), check their advisories and update those tools if they ship a vulnerable go-getter. [2][3]
  • Run dependency scans (SCA) against your builds/containers to find any vendored go-getter <=1.7.8 and remediate.

References

  1. HashiCorp advisory (HCSEC-2025-23) — go-getter fixed in 1.7.9. [HashiCorp advisory]. [1]
  2. NVD entry for CVE-2025-8959. [2]
  3. GitLab advisory / package advisories summarizing affected versions and fix. [3]

Would you like me to scan a specific binary/container image, repo, or list of tools for the vulnerable go-getter dependency?


🌐 Web query:

CVE-2025-61729 kubectl vulnerability

💡 Result:

Short summary — what it is

  • CVE-2025-61729 is a Go crypto/x509 issue where HostnameError.Error() can print an unbounded list of hosts using repeated string concatenation, causing excessive CPU/memory (DoS) when a maliciously-crafted certificate is presented. [1][2]

Impact for kubectl

  • Any Go-built binary that uses crypto/x509 Certificate.Verify / VerifyHostname is potentially affected — that includes kubectl builds that were compiled with vulnerable Go versions. An affected kubectl could be made to consume excessive resources (denial of service) by a crafted certificate. [1][2]

Affected versions / fix

  • Affects Go: before go1.24.11, and from go1.25.0 before go1.25.5. The fix was applied in the Go project (see patch/issue). [2][3]

What you should do

  • If you run kubectl (or other Go binaries) compiled with a vulnerable Go version, update to a kubectl build that was compiled with a fixed Go (i.e., use vendor/distribution kubectl releases rebuilt with Go ≥ go1.24.11 or ≥ go1.25.5). If you build kubectl yourself, upgrade your Go toolchain to a fixed release and rebuild. Monitor vendor/security advisories for your Kubernetes distribution for patched kubectl packages. [2][3]

References

  • NVD entry for CVE-2025-61729. [1]
  • Go vulnerability advisory (GO-2025-4155) — affected symbols and fixed versions. [2]
  • Go issue / patch tracking the fix. [3]

Sources:
[1] NVD — CVE-2025-61729.
[2] GO-2025-4155 (pkg.go.dev vulnerability report).
[3] golang issue / patch (Go project).


🌐 Web query:

CVE-2025-22868 kubectl Go dependency vulnerability

💡 Result:

Summary

  • CVE-2025-22868 is a vulnerability in golang.org/x/oauth2/jws (token parsing) that can cause excessive memory consumption (DoS). [1]
  • Affects versions of golang.org/x/oauth2/jws before v0.27.0; fixed in v0.27.0. [1]

Relevance to kubectl

  • The CVE is in a Go oauth2 library used by many Go projects and downstream components (e.g., gke-gcloud-auth-plugin, kubernetes-dashboard packages and other tools). If your kubectl or its exec-plugins (like gke-gcloud-auth-plugin) or other auth-related binaries were built linking a vulnerable oauth2 version, they may be affected. A Kubernetes repo issue and multiple downstream advisories discuss updating the client/deps. [2][3]

Recommended actions

  1. Identify binaries/plugins in your environment that use golang.org/x/oauth2 (kubectl plugins, gke-gcloud-auth-plugin, dashboards, cloud agents, etc.).
  2. Update affected components to use oauth2 v0.27.0 or later (or install vendor/OS vendor patches). For example, update gke-gcloud-auth-plugin / kubectl builds or install vendor security updates. [1][3]
  3. Rebuild/redeploy binaries that statically include the vulnerable module, or install vendor/OS patches where provided. Monitor vendor advisories for fixed packages. [1][3]

References
[1] Go vulnerability report (GO-2025-3488 / CVE-2025-22868) — pkg.go.dev.
[2] kubernetes/kubernetes issue discussing CVE-2025-22868.
[3] Snyk advisory examples (gke-gcloud-auth-plugin / kubernetes-dashboard-api) and vendor advisories.


kubectl installation copies single binary to version-specific directories, and CVE claims in comment need clarification.

The code downloads one "stable" version of kubectl but copies it to directories named 1.30 and 1.31. This is misleading—if any tooling expects version-specific kubectl binaries at these paths, it will silently get the wrong version.

The CVE claims in the comment are also overstated:

  • CVE-2025-8959 does not affect upstream kubectl directly (only tools that vendor the go-getter library).
  • CVE-2025-61729 and CVE-2025-22868 may affect kubectl only if it was compiled with specific vulnerable Go versions or oauth2 dependency versions—simply installing "latest" does not guarantee these are fixed unless you verify the build metadata.

Either remove the unverified CVE claims or update the comment to reflect the actual conditions under which they apply. For the version-specific directory issue, if separate 1.30 and 1.31 binaries are genuinely needed, install each version explicitly:

🔧 Suggested fix for version-specific kubectl
-    # Install latest kubectl versions to fix CVE-2025-8959, CVE-2025-61729, and CVE-2025-22868
-    # These vulnerabilities affect kubectl binaries built with vulnerable Go dependencies
-    # Installing latest kubectl versions that were built with fixed Go 1.22.6+ and updated dependencies
-    KUBECTL_VERSION=$(curl -L -s https://dl.k8s.io/release/stable.txt) && \
-    mkdir -p /opt/kubectl/1.30 /opt/kubectl/1.31 && \
-    curl -LO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" && \
-    curl -LO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl.sha256" && \
-    echo "$(cat kubectl.sha256)  kubectl" | sha256sum --check && \
-    chmod +x kubectl && \
-    cp kubectl /opt/kubectl/1.30/kubectl && \
-    cp kubectl /opt/kubectl/1.31/kubectl && \
-    cp kubectl /usr/local/bin/kubectl && \
-    rm kubectl kubectl.sha256 && \
+    # Install version-specific kubectl binaries
+    mkdir -p /opt/kubectl/1.30 /opt/kubectl/1.31 && \
+    # Install kubectl 1.30 (latest patch)
+    KUBECTL_130=$(curl -L -s https://dl.k8s.io/release/stable-1.30.txt) && \
+    curl -LO "https://dl.k8s.io/release/${KUBECTL_130}/bin/linux/amd64/kubectl" && \
+    curl -LO "https://dl.k8s.io/release/${KUBECTL_130}/bin/linux/amd64/kubectl.sha256" && \
+    echo "$(cat kubectl.sha256)  kubectl" | sha256sum --check && \
+    chmod +x kubectl && mv kubectl /opt/kubectl/1.30/kubectl && rm kubectl.sha256 && \
+    # Install kubectl 1.31 (latest patch)
+    KUBECTL_131=$(curl -L -s https://dl.k8s.io/release/stable-1.31.txt) && \
+    curl -LO "https://dl.k8s.io/release/${KUBECTL_131}/bin/linux/amd64/kubectl" && \
+    curl -LO "https://dl.k8s.io/release/${KUBECTL_131}/bin/linux/amd64/kubectl.sha256" && \
+    echo "$(cat kubectl.sha256)  kubectl" | sha256sum --check && \
+    chmod +x kubectl && mv kubectl /opt/kubectl/1.31/kubectl && rm kubectl.sha256 && \
+    # Symlink default kubectl to 1.31
+    ln -s /opt/kubectl/1.31/kubectl /usr/local/bin/kubectl && \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Install latest kubectl versions to fix CVE-2025-8959, CVE-2025-61729, and CVE-2025-22868
# These vulnerabilities affect kubectl binaries built with vulnerable Go dependencies
# Installing latest kubectl versions that were built with fixed Go 1.22.6+ and updated dependencies
KUBECTL_VERSION=$(curl -L -s https://dl.k8s.io/release/stable.txt) && \
mkdir -p /opt/kubectl/1.30 /opt/kubectl/1.31 && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl" && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl.sha256" && \
echo "$(cat kubectl.sha256) kubectl" | sha256sum --check && \
chmod +x kubectl && \
cp kubectl /opt/kubectl/1.30/kubectl && \
cp kubectl /opt/kubectl/1.31/kubectl && \
cp kubectl /usr/local/bin/kubectl && \
rm kubectl kubectl.sha256 && \
# Install version-specific kubectl binaries
mkdir -p /opt/kubectl/1.30 /opt/kubectl/1.31 && \
# Install kubectl 1.30 (latest patch)
KUBECTL_130=$(curl -L -s https://dl.k8s.io/release/stable-1.30.txt) && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_130}/bin/linux/amd64/kubectl" && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_130}/bin/linux/amd64/kubectl.sha256" && \
echo "$(cat kubectl.sha256) kubectl" | sha256sum --check && \
chmod +x kubectl && mv kubectl /opt/kubectl/1.30/kubectl && rm kubectl.sha256 && \
# Install kubectl 1.31 (latest patch)
KUBECTL_131=$(curl -L -s https://dl.k8s.io/release/stable-1.31.txt) && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_131}/bin/linux/amd64/kubectl" && \
curl -LO "https://dl.k8s.io/release/${KUBECTL_131}/bin/linux/amd64/kubectl.sha256" && \
echo "$(cat kubectl.sha256) kubectl" | sha256sum --check && \
chmod +x kubectl && mv kubectl /opt/kubectl/1.31/kubectl && rm kubectl.sha256 && \
# Symlink default kubectl to 1.31
ln -s /opt/kubectl/1.31/kubectl /usr/local/bin/kubectl && \
🤖 Prompt for AI Agents
In @deployer-image/Dockerfile around lines 47 - 59, The Dockerfile currently
fetches a single "stable" kubectl (KUBECTL_VERSION) but then copies that binary
into versioned paths (/opt/kubectl/1.30 and /opt/kubectl/1.31), which is
misleading; either download and verify each desired version explicitly (fetch
distinct URLs/checksums per version and place them into
/opt/kubectl/1.30/kubectl and /opt/kubectl/1.31/kubectl, keeping the existing
sha256 verification per file) or stop using versioned directories and name them
/opt/kubectl/stable to match KUBECTL_VERSION, and update the cp
/usr/local/bin/kubectl step accordingly; also update the top comment about CVEs
to remove blanket claims and instead state the exact conditions (e.g., “these
CVEs may affect binaries built with vulnerable Go/oauth2/go-getter versions;
ensure the downloaded kubectl build metadata or Go toolchain is fixed, or verify
by using explicit versioned releases”) so the comment accurately reflects when
those CVEs apply.

Comment on lines +60 to +61
# Upgrade pip to latest version to fix CVE-2024-35195 and CVE-2025-47273
python3 -m pip install --upgrade pip setuptools wheel --no-cache-dir && \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

CVE-2024-35195 vulnerability

💡 Result:

Summary

  • CVE-2024-35195: a bug in the Python Requests library (fixed in requests 2.32.0) where if the first request made by a Session to a host sets verify=False, subsequent requests to that host in the same connection pool may also skip TLS certificate verification, exposing connections to MITM risk. [1][2][3]

Affected versions

  • Requests prior to 2.32.0. Fixed in requests 2.32.0. [1][2]

Impact

  • TLS verification can be unintentionally bypassed for the lifetime of a pooled connection, risking confidentiality and integrity (possible MITM). Severity rated medium (CVSS ~5.6–6.5 by different vendors). [1][3][4]

Mitigation and workarounds

  • Upgrade to requests>=2.32.0 (recommended). [1][2]
  • If you cannot upgrade immediately: avoid making the first request to a host with verify=False when using a Session; or call Session.close() (or otherwise close/reset connections) after any request that used verify=False to clear the connection pool. [2][4]

References
[1] NVD summary for CVE-2024-35195.
[2] AWS / Amazon Linux advisory entry for CVE-2024-35195.
[3] Ubuntu security notice for CVE-2024-35195 (notes and patches).
[4] Wiz vulnerability summary / mitigations for CVE-2024-35195.

Would you like exact upgrade commands or a quick script to detect vulnerable requests versions across your environment?


🌐 Web query:

CVE-2025-47273 vulnerability

💡 Result:

Summary — CVE-2025-47273

  • What it is: a path‑traversal vulnerability in setuptools' PackageIndex that can allow writing files to arbitrary filesystem locations (possibly leading to RCE depending on context). [1][2]
  • Affected versions: setuptools prior to 78.1.1. [1][2]
  • Fix: upgrade setuptools to version 78.1.1 or later. [2]
  • Severity/impact: rated high; arbitrary file write as the process user (attack surface depends on how setuptools is used in your environment). [1][3]
  • Mitigations if you cannot upgrade immediately: restrict execution privileges of processes that run packaging code, restrict network access to untrusted package indexes, and validate/sandbox build environments. [2][4]

References
[1] NVD entry for CVE-2025-47273.
[2] GitHub setuptools security advisory / patch (fix in v78.1.1).
[3] Ubuntu security notice for CVE-2025-47273.
[4] AWS/ALAS advisory for CVE-2025-47273.


Fix CVE attribution in the Dockerfile comment.

CVE-2024-35195 is a Python Requests library vulnerability (TLS verification bypass), not a pip vulnerability. Upgrading pip does not address this CVE. CVE-2025-47273 is a legitimate setuptools vulnerability (path-traversal), so upgrading setuptools is appropriate, but the Dockerfile comment incorrectly attributes both CVEs to the pip upgrade. Revise the comment to either:

  • Reference only CVE-2025-47273 for the setuptools upgrade, or
  • Add a separate pip/requests upgrade step if CVE-2024-35195 remediation is intended (upgrade requests>=2.32.0)
🤖 Prompt for AI Agents
In @deployer-image/Dockerfile around lines 60 - 61, The Dockerfile comment
incorrectly attributes CVE-2024-35195 to pip; update the comment above the
existing python3 -m pip install --upgrade pip setuptools wheel --no-cache-dir
line to state that upgrading setuptools addresses CVE-2025-47273 and does not
remediate CVE-2024-35195, and if you intend to remediate CVE-2024-35195 also,
add a separate pip install step to upgrade the requests package (e.g., install
requests>=2.32.0) after the existing upgrade; ensure the comment and/or added
step clearly distinguishes which package addresses which CVE.

Comment on lines +78 to +80
# Enable debug logging by default to help diagnose issues
# Set to "false" to disable verbose output and Terraform debug logs
ENV DEBUG=true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

DEBUG=true by default may expose sensitive information.

Enabling debug logging by default in a deployer image risks leaking sensitive data (Terraform state, secrets, credentials) in logs. Consider defaulting to false and requiring explicit opt-in for debugging.

🔧 Suggested fix
 # Enable debug logging by default to help diagnose issues
-# Set to "false" to disable verbose output and Terraform debug logs
-ENV DEBUG=true
+# Set to "true" to enable verbose output and Terraform debug logs
+ENV DEBUG=false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Enable debug logging by default to help diagnose issues
# Set to "false" to disable verbose output and Terraform debug logs
ENV DEBUG=true
# Enable debug logging by default to help diagnose issues
# Set to "true" to enable verbose output and Terraform debug logs
ENV DEBUG=false
🤖 Prompt for AI Agents
In @deployer-image/Dockerfile around lines 78 - 80, The Dockerfile currently
sets ENV DEBUG=true which enables verbose logging by default and risks leaking
secrets; change the default to false and require explicit opt-in (e.g.,
introduce ARG DEBUG=false and set ENV DEBUG from that ARG or change ENV DEBUG to
"false"), update any documentation/CI/build scripts that rely on DEBUG to
explicitly pass true when needed, and ensure callers know to set DEBUG=true only
for troubleshooting to avoid accidental exposure.

Comment on lines +6 to 8
publishedVersion: "4.1.2"
publishedVersionMetadata:
releaseNote: "Initial release with Job support."
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "schema.yaml" | head -20

Repository: appcd-dev/deployer

Length of output: 153


🏁 Script executed:

find . -type f \( -name "*CHANGE*" -o -name "*RELEASE*" -o -name "*VERSION*" \) | grep -v node_modules | head -20

Repository: appcd-dev/deployer

Length of output: 44


🏁 Script executed:

git ls-files | grep -E "(deployer-image|marketplace)" | head -30

Repository: appcd-dev/deployer

Length of output: 1550


🏁 Script executed:

cat -n deployer-image/marketplace/schema.yaml

Repository: appcd-dev/deployer

Length of output: 4871


🏁 Script executed:

git log --oneline --all deployer-image/marketplace/schema.yaml | head -20

Repository: appcd-dev/deployer

Length of output: 170


🏁 Script executed:

find . -type f \( -name "*.md" -o -name "README*" \) | xargs grep -l "Job support\|publishedVersion\|4\.1\.2" 2>/dev/null | head -10

Repository: appcd-dev/deployer

Length of output: 44


🏁 Script executed:

git show bb85b3a:deployer-image/marketplace/schema.yaml | head -15

Repository: appcd-dev/deployer

Length of output: 566


🏁 Script executed:

git show 7dce41e:deployer-image/marketplace/schema.yaml | head -15

Repository: appcd-dev/deployer

Length of output: 566


🏁 Script executed:

git show a3734ea:deployer-image/marketplace/schema.yaml | head -15

Repository: appcd-dev/deployer

Length of output: 487


🏁 Script executed:

git show 3db2647:deployer-image/marketplace/schema.yaml | head -15

Repository: appcd-dev/deployer

Length of output: 487


Update releaseNote to reflect version 4.1.2.

The release note "Initial release with Job support." has not been updated since the initial commit, while publishedVersion has progressed through 1.0.5 → 1.0.0 → 2.0.0 → 4.1.2. Version 4.1.2 is not an initial release; update the release note to accurately describe what this version contains or represents.

🤖 Prompt for AI Agents
In @deployer-image/marketplace/schema.yaml around lines 6 - 8, The releaseNote
under publishedVersionMetadata still reads "Initial release with Job support."
but publishedVersion is "4.1.2"; update publishedVersionMetadata.releaseNote to
accurately describe what changed or is included in v4.1.2 (e.g., features, bug
fixes, or API changes), ensuring the text references version 4.1.2 and
summarizes its actual contents rather than saying "Initial release."

Comment on lines +4 to +8
# Enable debug mode if DEBUG environment variable is set
if [ "${DEBUG:-false}" = "true" ]; then
set -x
export TF_LOG=INFO
fi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n deployer-image/scripts/deploy.sh | head -100

Repository: appcd-dev/deployer

Length of output: 4612


🏁 Script executed:

# Check if STACKGEN_PAT is indeed passed as CLI arg
rg -A 5 -B 5 'terraform apply' deployer-image/scripts/deploy.sh

Repository: appcd-dev/deployer

Length of output: 414


🏁 Script executed:

# Check the values file format
find . -name "*values*" -o -name "*values.yaml" | head -20

Repository: appcd-dev/deployer

Length of output: 152


🏁 Script executed:

# Verify terraform variable naming conventions
rg 'variable.*STACKGEN_PAT|variable.*stackgen_pat' --type tf --type hcl

Repository: appcd-dev/deployer

Length of output: 89


🏁 Script executed:

find deployer-image/terraform -name "*.tf" | head -10

Repository: appcd-dev/deployer

Length of output: 494


🏁 Script executed:

rg 'variable' deployer-image/terraform --type tf -A 2

Repository: appcd-dev/deployer

Length of output: 6387


🏁 Script executed:

# Check if terraform variables.tf or main.tf exist
ls -la deployer-image/terraform/

Repository: appcd-dev/deployer

Length of output: 608


🏁 Script executed:

# Verify terraform supports TF_VAR_* environment variables
web_search "terraform TF_VAR environment variables CLI arguments"

Repository: appcd-dev/deployer

Length of output: 112


🏁 Script executed:

# Look for all variables passed to terraform apply
rg 'var "' deployer-image/scripts/deploy.sh -A 1 -B 1

Repository: appcd-dev/deployer

Length of output: 669


🏁 Script executed:

# Check if TF_LOG or other Terraform environment variables are used
rg 'TF_VAR|TF_LOG' deployer-image/scripts/deploy.sh

Repository: appcd-dev/deployer

Length of output: 82


🌐 Web query:

terraform TF_VAR environment variable alternative to CLI arguments

💡 Result:

Short answer — yes. Alternatives to using TF_VAR_... environment variables:

  • Pass variables on the CLI with -var or -var-file (highest precedence). [1]
  • Use variable definition files: terraform.tfvars, terraform.tfvars.json, or *.auto.tfvars (auto-loaded). [1]
  • Set default values in the variable block. [1]
  • Use TF_CLI_ARGS or TF_CLI_ARGS_name to inject CLI flags (e.g., -var-file) from an environment variable. [2]
  • For Terraform Cloud/Enterprise, set workspace variables or use HCP/Terraform Cloud variable sets. [1][3]
  • If you need to read arbitrary environment variables into config, use the external data source to pass them into Terraform. [4]

Notes: remote backends / Terraform Cloud runs can restrict using -var directly in some contexts — prefer auto.tfvars or workspace variables there. [3]

Sources: [1] HashiCorp Terraform variables docs; [2] Terraform env vars / TF_CLI_ARGS docs; [3] HashiCorp support on -var and remote backends; [4] HashiCorp support article on reading env vars via external data source.


🌐 Web query:

terraform TF_VAR_variable_name environment variable naming convention

💡 Result:

Use the TF_VAR_ prefix followed immediately by the Terraform variable name: TF_VAR_<variable_name>. Environment variable names must match the variable name exactly (including case on case-sensitive OSes). For complex types pass a JSON/HCL string (or prefer .tfvars files); TF_VARs are lower-precedence than CLI -var and tfvars files. (developer.hashicorp.com)

Citations:


DEBUG set -x will leak STACKGEN_PAT to logs when terraform apply runs with CLI-passed variables.

With set -x enabled (line 6), the terraform apply command (lines 61–67) will echo the entire command with expanded variables to stderr, exposing the secret. This also occurs in the error-handling path (line 126) where terraform plan passes the same secret.

Pass STACKGEN_PAT via TF_VAR_STACKGEN_PAT environment variable instead of -var CLI argument. Temporarily disable set -x around the export to keep the secret out of debug output:

Proposed fix
 echo "[INFO] Applying Terraform configuration"
 echo "[DEBUG] Terraform version: $(terraform version | head -1)"
+
+# Use environment variable for secrets to avoid CLI-arg leakage with set -x
+if [ "${DEBUG:-false}" = "true" ]; then set +x; fi
+export TF_VAR_STACKGEN_PAT="$STACKGEN_PAT"
+if [ "${DEBUG:-false}" = "true" ]; then set -x; fi
+
 terraform apply \
   -var "suffix=${SUFFIX}" \
   -var "domain=${DOMAIN}" \
-  -var "STACKGEN_PAT=${STACKGEN_PAT}" \
   -var "pre_shared_cert_name=${PRE_SHARED_CERT_NAME}" \
   -var "global_static_ip_name=${GLOBAL_STATIC_IP_NAME}" \
   -auto-approve

Apply the same fix to the error path (line 126) where terraform plan is called.

🤖 Prompt for AI Agents
In @deployer-image/scripts/deploy.sh around lines 4 - 8, The script enables
shell debug (set -x) which will leak STACKGEN_PAT when terraform commands echo
CLI args; to fix, stop exporting or revealing the secret by: temporarily disable
xtrace around exporting/setting the secret (use "set +x" before and "set -x"
after the export) and stop passing STACKGEN_PAT via a -var CLI argument in the
terraform apply and terraform plan invocations—instead set TF_VAR_STACKGEN_PAT
as an environment variable (export TF_VAR_STACKGEN_PAT="$STACKGEN_PAT") so the
secret is injected into Terraform without appearing in the command line; apply
the same temporary set +x / set -x protection and TF_VAR_STACKGEN_PAT change for
both the terraform apply block and the terraform plan error-handling path.

Comment on lines +91 to +103
for pod in $(kubectl get pods -n stackgen -o jsonpath='{.items[*].metadata.name}' 2>/dev/null || echo ""); do
if [ -n "$pod" ]; then
phase=$(kubectl get pod -n stackgen "$pod" -o jsonpath='{.status.phase}' 2>&1 || echo "Unknown")
ready=$(kubectl get pod -n stackgen "$pod" -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}' 2>&1 || echo "Unknown")
echo "[INFO] Pod: $pod | Phase: $phase | Ready: $ready"

# If pod is not ready, show why
if [ "$ready" != "True" ] && [ "$phase" != "Running" ]; then
echo "[WARNING] Pod $pod is not ready. Showing details:"
kubectl describe pod -n stackgen "$pod" 2>&1 | grep -A 10 "Status:\|Events:" | tee /dev/stderr || true
echo "[WARNING] Last 30 lines of logs from $pod:"
kubectl logs -n stackgen "$pod" --tail=30 2>&1 | tee /dev/stderr || true
fi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and read the relevant lines
if [ -f "deployer-image/scripts/deploy.sh" ]; then
  echo "File found. Reading lines 91-103:"
  sed -n '91,103p' deployer-image/scripts/deploy.sh
else
  echo "File not found at deployer-image/scripts/deploy.sh"
  # Try to find it
  find . -name "deploy.sh" -type f 2>/dev/null | head -20
fi

Repository: appcd-dev/deployer

Length of output: 1022


Change && to || on line 98 to diagnose Running-but-not-Ready pods.
With the current && operator, diagnostics are skipped when phase=Running but ready!=True (e.g., CrashLoopBackOff, failing readiness probe)—a common failure mode that needs debugging. Use || instead so diagnostics run whenever the pod is either not Ready OR not Running.

Proposed fix
-          if [ "$ready" != "True" ] && [ "$phase" != "Running" ]; then
+          if [ "$ready" != "True" ] || [ "$phase" != "Running" ]; then
             echo "[WARNING] Pod $pod is not ready. Showing details:"
             kubectl describe pod -n stackgen "$pod" 2>&1 | grep -A 10 "Status:\|Events:" | tee /dev/stderr || true
             echo "[WARNING] Last 30 lines of logs from $pod:"
             kubectl logs -n stackgen "$pod" --tail=30 2>&1 | tee /dev/stderr || true
           fi
🤖 Prompt for AI Agents
In @deployer-image/scripts/deploy.sh around lines 91 - 103, The pod readiness
check in deploy.sh incorrectly uses a logical AND between "$ready" and "$phase",
so diagnostics only run when the pod is neither Ready nor Running; change the
condition in the if that checks ready and phase to use OR (i.e., replace && with
||) so that when either ready != "True" OR phase != "Running" the script prints
describe/logs for the pod (references: variables pod, phase, ready and the if
block that echoes "[WARNING] Pod $pod is not ready. Showing details:").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants